-
Notifications
You must be signed in to change notification settings - Fork 598
util: disable informers on secrets #5984
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This patch disables the secret cache which relied on Informers. As the informer was cluster scoped, it led to huge memory spikes due to decoding every secret. The cache is disabled for it to be implemented in a better manner or to be removed entirely later. Signed-off-by: Niraj Yadav <[email protected]>
8a70211 to
cdbe6cb
Compare
Rakshith-R
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
|
@Mergifyio rebase |
☑️ Nothing to do, the required conditions are not metDetails
|
|
@Mergifyio queue |
❌ The pull request has been removed from the queue
|
Merge Queue Status🚫 The pull request has left the queue (rule: This pull request spent 34 minutes 37 seconds in the queue, including 34 minutes 26 seconds running CI. Required conditions to merge
ReasonThe merge conditions cannot be satisfied due to failing checks Failing checks: HintYou may have to fix your CI before adding the pull request to the queue again. |
|
@black-dragon74 , has it been confirmed that this addressees rook/rook#16962 ? Please create an issue for re-enabling the informers. There is no need to keep the old function available, it is all in the git history. |
Yes. It will. There was no leak but rather too much allocation by the secret watcher.
Done at: #5985, let’s keep the function for now as the majority of it will be reused, soon. |
|
@Mergifyio requeue |
✅ This pull request will be re-embarked automaticallyDetailsThe followup |
❌ The pull request has been removed from the queue
|
Merge Queue Status🚫 The pull request has left the queue (rule: This pull request spent 2 hours 48 minutes 5 seconds in the queue, including 2 hours 47 minutes 55 seconds running CI. Required conditions to merge
ReasonThe merge conditions cannot be satisfied due to failing checks Failing checks: HintYou may have to fix your CI before adding the pull request to the queue again. |
|
/test ci/centos/mini-e2e/k8s-1.33 |
|
@Mergifyio requeue |
✅ This pull request will be re-embarked automaticallyDetailsThe followup |
✅ The pull request has been merged automaticallyDetailsThe pull request has been merged automatically at fb4fb8f |
Merge Queue Status✅ The pull request has been merged at cdbe6cb This pull request spent 3 hours 2 minutes 58 seconds in the queue, including 3 hours 2 minutes 39 seconds running CI. Required conditions to merge
|
Describe what this PR does
This patch disables the secret cache which relied on Informers. As the informer was cluster scoped, it led to huge memory spikes due to decoding every secret.
The cache is disabled for it to be implemented in a better manner or to be removed entirely later.